-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(rich-text-editor): DLT-2116 enable emoji searching by name and keyword #1
base: staging
Are you sure you want to change the base?
Conversation
Signed-off-by: Julio Ortega <[email protected]>
Signed-off-by: Julio Ortega <[email protected]>
Signed-off-by: Julio Ortega <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Enhanced emoji suggestion functionality in the rich text editor for both Vue 2 and Vue 3 versions.
- Added keyword and name-based emoji searching in
packages/dialtone-vue2/components/rich_text_editor/extensions/emoji/suggestion.js
and Vue 3 equivalent - Implemented a 20-result suggestion limit to improve performance
- Updated filtering logic to accommodate new search capabilities
- Changes are identical in both Vue 2 and Vue 3 files
2 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings
return false; | ||
}); | ||
return filteredEmoji.map(item => { return { code: item.shortname }; }); | ||
query = query.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Reassigning function parameter. Consider using a new variable name
const filteredEmoji = emojiList | ||
.filter( | ||
item => [ | ||
item.name, | ||
item.shortname.replaceAll(':', ''), | ||
...item.keywords, | ||
].some(text => text.startsWith(query)), | ||
).splice(0, suggestionLimit); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: This filtering could be expensive for large emoji lists. Consider memoization or indexing for better performance
.filter( | ||
item => [ | ||
item.name, | ||
item.shortname.replaceAll(':', ''), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: replaceAll might not be supported in all environments. Consider using replace with regex
return false; | ||
}); | ||
return filteredEmoji.map(item => { return { code: item.shortname }; }); | ||
query = query.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Reassigning function parameter. Consider using a new variable name
const filteredEmoji = emojiList | ||
.filter( | ||
item => [ | ||
item.name, | ||
item.shortname.replaceAll(':', ''), | ||
...item.keywords, | ||
].some(text => text.startsWith(query)), | ||
).splice(0, suggestionLimit); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: This filtering logic might be computationally expensive for large emoji lists. Consider optimizing if performance issues arise
.filter( | ||
item => [ | ||
item.name, | ||
item.shortname.replaceAll(':', ''), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: replaceAll may not be supported in all browsers. Consider using replace with a regex for better compatibility
Enable emoji searching by name and keyword
Obligatory GIF (super important!)
🛠️ Type Of Change
These types will increment the version number on release:
📖 Jira Ticket
https://dialpad.atlassian.net/browse/DLT-2116
📖 Description
suggestionLimit
to limit the results to 20, this hugely improves the performance.💡 Context
📝 Checklist
For all PRs:
For all Vue changes:
./scripts/dialtone-vue-sync.sh
script. Read docs here: Dialtone Vue Sync Script🔮 Next Steps
Release, update and test on product
📷 Screenshots / GIFs